Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip computing assets summary for invalid assets #1489

Merged
merged 1 commit into from
Feb 13, 2023

Conversation

danlamanna
Copy link
Contributor

This part of the code should have been ported over during #1431.

@satra
Copy link
Member

satra commented Feb 13, 2023

i think this needs to be a more critical response than an avoidance. i.e. we need to fix when this happens. so i would suggest not merging this, but instead using something like this to add to the admin dashboard, so that we know what to fix. the silent suppression that this does is not going to be helpful.

@danlamanna
Copy link
Contributor Author

I'm merging this for now because all it does is restore the status quo that existed up until a few days ago.

so i would suggest not merging this, but instead using something like this to add to the admin dashboard, so that we know what to fix.

It seems like we already track invalid assets, we just need to improve the dashboard for it (#1197). Are you asking for something different? In general, I would like it if dandi-schema provided a better way of distinguishing between genuinely exceptional events and problems with client provided metadata (dandi/dandi-schema#127 (comment)).

@danlamanna danlamanna added patch Increment the patch version when merged release Create a release when this pr is merged labels Feb 13, 2023
@danlamanna danlamanna merged commit e538de6 into master Feb 13, 2023
@danlamanna danlamanna deleted the ignore-assets-summary-failures branch February 13, 2023 16:13
@dandibot
Copy link
Member

🚀 PR was released in v0.3.15 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Feb 13, 2023
@satra
Copy link
Member

satra commented Feb 13, 2023

i think this would result, for any landing page to not show anything in the asset summary. this would only be detected when someone goes on the landing page, rather than fixing the core issue of assets. hence i wanted a more severe restriction that when this happens, we should, as dandi, fix the issue as fast as possible. in most cases this is not a user issue. more often a dandi issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged release Create a release when this pr is merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants